feat: add prime total rewards card#5626
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR implements the Prime Total Rewards and User Rewards cards for the Prime Leaderboard page, adding per-market reward rows with progress bars, a
Confidence Score: 5/5Safe to merge; all findings are style and architecture suggestions with no functional defects in the changed code paths. All reward card logic is straightforward — placeholder data with explicit TODOs, the zero-total guard is present, the modal button is wired up, and wallet-gating for the user section is correctly implemented. The open items (type-import direction, raw hex colour, pool-fetch placement) are clean-up concerns that do not affect correctness or user-visible behaviour at this stage. UserRewardsCard/index.tsx and the two placeholder hooks (useGetPrimeTotalRewards, useGetPrimeUserRewards) — the type-import coupling and the direct pool-fetch inside the card should be cleaned up before the real API is wired in. Important Files Changed
Reviews (5): Last reviewed commit: "fix: review comments" | Re-trigger Greptile |
| <div className="h-1.5 w-18 overflow-hidden rounded-full bg-lightGrey"> | ||
| <div | ||
| className="h-full rounded-full bg-green" | ||
| style={{ width: `${Math.min(100, (rewardsCents / totalRewardsCents) * 100)}%` }} |
There was a problem hiding this comment.
When
totalRewardsCents is 0, the expression rewardsCents / totalRewardsCents produces either Infinity (when rewardsCents > 0) or NaN (when both are 0). Math.min(100, Infinity) resolves correctly to 100, but Math.min(100, NaN) returns NaN, making the inline style width: NaN% — an invalid CSS value that the browser silently ignores, leaving the bar collapsed.
| style={{ width: `${Math.min(100, (rewardsCents / totalRewardsCents) * 100)}%` }} | |
| style={{ | |
| width: `${totalRewardsCents > 0 ? Math.min(100, (rewardsCents / totalRewardsCents) * 100) : 0}%`, | |
| }} |
There was a problem hiding this comment.
Fixed — added a zero-total guard so the bar renders at 0% instead of producing a NaN width.
| const marketRewards = tokens | ||
| .slice(0, placeholderMarketRewardsCents.length) | ||
| .map((token, index) => ({ token, rewardsCents: placeholderMarketRewardsCents[index] })); |
There was a problem hiding this comment.
Placeholder tokens are unrelated to Prime markets
useGetTokens() returns all tokens registered in the app's token list, and .slice(0, 2) picks whichever two happen to be first — typically generic tokens like BTC/ETH that have nothing to do with Prime reward markets. The card will render the wrong token icons and symbols paired with arbitrary cent values, actively misleading users. Even as temporary placeholder data, pairing hardcoded amounts with arbitrary tokens is likely to produce a confusing UI on any real environment. Consider substituting a static fallback (e.g. hardcoded token stubs or an empty array) until the real API hook is wired up.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
These are placeholder tokens until the API hook lands; added a TODO noting they're unrelated to the real Prime markets and will be replaced.
|
@greptile review again |
62bff1f to
4f5780c
Compare
|
@greptile review again |
e6cf09b to
7b2c2d9
Compare
7da6c3a to
2131c0f
Compare
There was a problem hiding this comment.
What do you think about moving this component to the containers directory and reusing it in the MarketTable component (https://github.com/VenusProtocol/venus-protocol-interface/blob/088de2b0451f8a4ef5364cbaa7e3d2133b4d9bc8/apps/evm/src/containers/MarketTable/index.tsx) which renders the same UI using the renderRowControl property?
There was a problem hiding this comment.
MarketTable and the Prime button use it now. I kept the buttons separate rather than reusing the Prime button as MarketTable's renderRowControl, because MarketTable's row click goes through a gated-asset acknowledgement flow (GatedAssetAcknowledgementModal) before opening the form — reusing the self-contained Prime button would bypass that.
Or do we need move the gated logic into the shared component?
There was a problem hiding this comment.
Interesting. I think we need to move the gating logic too, because it would also apply in the case of the Prime leaderboard.
So I'd say that MarketFormModal should also be in charge of checking if the user has accepted the conditions for using gated assets and display the acknowledgement modal (GatedAssetAcknowledgementModal) instead of the form modal if not.
There was a problem hiding this comment.
Make sense. let me handle this logic
There was a problem hiding this comment.
moved the gating into MarketFormModal itself, so it now checks isGated against the user's acknowledgement and shows GatedAssetAcknowledgementModal instead of the form when needed. MarketTable no longer handles it, and the Prime leaderboard's MarketActionsButton now gets the gating
7b2c2d9 to
a321c48
Compare
6eef450 to
56f19c9
Compare
|
@greptile review again |
|
PR looks good to me, only one comment left to address (about the gating logic for the market form modal) otherwise it's ready to be merged. |
@therealemjy thanks! addressed with - moved the gating into MarketFormModal itself, so it now checks isGated against the user's acknowledgement and shows GatedAssetAcknowledgementModal instead of the form when needed. MarketTable no longer handles it, and the Prime leaderboard's MarketActionsButton now gets the gating |
Drive the user rewards card headline by Prime eligibility (rewards amount, eligible-to-supply prompt, or not-eligible prompt) and render per-market Prime APYs with the shared Apy component. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
c8d0d87 to
6085a57
Compare
|
@cuzz-venus Very nice, approved. |
Jira ticket(s)
VPD-1333
Changes